Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add initial exact exchange implementation #942

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Conversation

mfherbst
Copy link
Member

@mfherbst mfherbst commented Jan 4, 2024

Small overhowl of #717.

mfherbst and others added 2 commits January 4, 2024 09:44
This was referenced Jan 4, 2024
@mfherbst mfherbst marked this pull request as ready for review January 4, 2024 12:44
@mfherbst
Copy link
Member Author

mfherbst commented Jan 10, 2024

@antoine-levitt Thoughts or blockers ?

"""
function model_HF(lattice::AbstractMatrix, atoms::Vector{<:Element},
positions::Vector{<:AbstractVector}; extra_terms=[], kwargs...)
@warn "Exact exchange in DFTK is hardly optimised and not yet production-ready."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

put that check in the instantiation of the exchange term?

@@ -0,0 +1,76 @@
@doc raw"""
Exact exchange term: the Hartree-Exact exchange energy of the orbitals
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calling it hartree is confusing?

# ABINIT/src/66_wfs/m_getghc.F90
# ABINIT/src/66_wfs/m_fock_getghc.F90
# ABINIT/src/66_wfs/m_prep_kgb.F90
# ABINIT/src/66_wfs/m_bandfft_kpt.F90
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather not give explicit references to code (as that might give the impression that we reproduced the implementation instead of a clean-room reimplementation)

# https://journals.aps.org/prb/pdf/10.1103/PhysRevB.73.205119
# https://journals.aps.org/prb/pdf/10.1103/PhysRevB.77.193110
# https://docs.abinit.org/topics/documents/hybrids-2017.pdf (Abinit apparently
# uses a short-ranged Coulomb)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that we should meet and think about, also with @ELallinec who is thinking about the Coulomb singularity

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely as a first attempt it's reasonable to have a short-ranged Coulomb.

@antoine-levitt
Copy link
Member

Have you tested this in any way? I'm reluctant to merge something that might just be flat out wrong, esp. with the Coulomb singularity. Maybe H2?

@antoine-levitt
Copy link
Member

Is this even converging at all? There's basically no damping on the Fock exchange by doing it this way. Direct minimization OTOH should work outside the box

@antoine-levitt
Copy link
Member

Roadmap after discussion:

  • be careful about Coulomb singularity (eg implement Gygi-Baldereschi correction as a first step)
  • make sure HF postprocessing (ie computing the exchange energy from an already converged LDA calculation) works
  • compare it to another code
  • implement self-consistency

@mfherbst mfherbst marked this pull request as draft August 5, 2024 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants